-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple Functions require lists of Feols/Feiv/Fepois object; reject FixestMulti #693 #715
Conversation
Convert FixestMulti to a list so that it works automatically with the following functions: - pf.bonferroni() - pf.rwolf() - pf.coefplot() - pf.iplot()
Codecov ReportAttention: Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Hi @IshwaraHegde97, looks very good already! Could you add small API test to test_multcomp.py? E.g. something like this: def test_multi_vs_list():
seed = 1232
data = pf.get_data()
fit_all = pf.feols("Y + Y2 ~ X1 + X2", data=data)
fit1 = pf.feols("Y ~ X1 + X2", data=data)
fit2 = pf.feols("Y2 ~ X1 + X2", data=data)
assert bonferroni(fit_all, "X1").equals(bonferroni([fit1, fit2], "X1"))
assert rwolf(fit_all, "X1", seed = seed).equals(rwolf([fit1, fit2], "X1", seed = seed)) |
Added a test to compare the results when passing a list of Feols/Feiv/Fepois objects vs. a FixestMulti object to bonferroni and rwolf functions.
pixi.lock file was updated
Minor changes for consistency
handle .coefplot() method for FixestMulti, where models is a dict_values instance.
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
@all-contributors please add @IshwaraHegde97 for code |
I've put up a pull request to add @IshwaraHegde97! 🎉 |
Added code to convert instances of FixestMulti into lists so that it automatically works with the following functions: